-
-
Notifications
You must be signed in to change notification settings - Fork 210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: createNavigation
#1316
Draft
amannn
wants to merge
38
commits into
canary
Choose a base branch
from
feat/create-navigation
base: canary
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
feat: createNavigation
#1316
+2,242
−167
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
15 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An exploration to create a unified function to create navigation APIs for an easier setup.
This is also intended to fix some long-standing issues with the current navigation APIs:
Link
forlocalePrefix: 'as-necessary'
and makegetPathname
more useful #444redirect
does not respectas-needed
locale prefix #1335TODO
react-server
react-client
domains
getPathname
)locale
as an argument for functions that generate pathnames?cloneElement
in RSC-rendered<Link />
(better compatibility with Radix Primitives & friends) #1271Link
: This helps to update the cookie. Theoretically we could do this in a click handler, but only after hydration. Should we remove the prefix as part of hydration?localePrefix
setting per domain #1055, at least enough to make sure this is possible to implement laterNew:
markers increateNavigation
Link
is now easierlocalePrefix: 'as-needed'
+domains
redirect
has to always include prefix since we don't know the host (can user providedomain
to avoid this? alternatively suggest multiple builds per domain)Link
prefixes always during SSR, but removes on client (can user providedomain
to avoid this?)unstable_
prefix?Improvements
redirect
no longer adds unnecessary locale prefixes forlocalePrefix
modes other thanalways
(fixesredirect
does not respectas-needed
locale prefix #1335)Link
getPathname
now requires adomain
when a combination oflocalePrefix: 'as-necessary'
anddomains
is used. This enables to compute the correct pathname based on thedefaultLocale
of a given domain.Migrating to
createNavigation
createNavigation
is generally a drop-in replacement forcreateSharedPathnamesNavigation
as well ascreateLocalizedPathnamesNavigation
.The following changes might be necessary in your code base though:
createLocalizedPathnamesNavigation
and have composed theLink
with itshref
prop, you no longer have to provide the genericPathname
type argument.getPathname
and have previously manually prepended a locale prefix, you should no longer do so—getPathname
now takes care of this depending on your routing strategy.